fix(admin): unblock webhook drains when subscribers stall by SebTardif · Pull Request #73 · openclaw/krillswitch

SebTardif · 2026-08-16T00:08:37Z

What Problem This Solves

Fixes post-mutation webhook drains hanging when a subscriber never responds.

User Impact

A timed-out attempt records timeout after two seconds, advances through that attempted entry, and lets other subscribers proceed. Unattempted entries stay queued for the next drain.

Why This Change Was Made

This preserves the notify-only delivery contract: attempted failures are not retried. The branch is updated to current main, with dashboard-operator documentation and an Unreleased entry. Thanks @SebTardif for the original fix; the contributor commits and co-author credit are preserved.

Evidence

The regression uses synthetic D1 fixtures without a competing background drain. It verifies one timed-out attempt, preservation of the next queued entry, delivery to a healthy subscriber, recovery, and no duplicate delivery. Against the original implementation it exceeds the three-second test limit; with the fix it passes. The complete gate on a clean AWS Crabbox install with Node 24.19.0 and pnpm 11.28.0 passes lint, typecheck, 277 tests, dashboard/demo/CLI builds, docs, and packed SDK verification. Independent Codex review is clean through P2.

No production endpoint was contacted. Exact-head CI and CodeQL passed at 192ef4db835ba7dde845f208367394359892e325. This PR awaits the required code-owner approval from the requested reviewers.

clawsweeper · 2026-08-16T00:08:41Z

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

clawsweeper · 2026-08-16T00:12:45Z

Codex review: needs real behavior proof before merge. Reviewed October 6, 2026, 11:59 PM ET / October 7, 2026, 03:59 UTC (Revision 33).

ClawSweeper review

What this changes

The PR bounds webhook notification attempts to two seconds, records timeouts, preserves untouched queued entries, and adds recovery coverage and operator documentation.

Merge readiness

⛔ Blocked before merge - 2 items remain

The fix remains necessary and no actionable code defect remains. The prior contract, regression-test, and changelog concerns are resolved; real transport proof through the production Worker drain remains incomplete.

Priority: P2
Reviewed head: 192ef4db835ba7dde845f208367394359892e325

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The focused patch now has sound regression coverage and documentation, but available runtime evidence still leaves the production transport path unproven.
Proof confidence	🦐 gold shrimp (3/6)	Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the updated D1 regression exercises drainWebhooks with simulated fetch clients, while preserved earlier Node cancellation evidence does not connect real transport to the Worker drain and remainder recovery. Provide redacted local Worker logs or terminal output showing timeout, persisted cursor, healthy-subscriber progress, and later untouched-entry delivery; production deployment is unnecessary. Screenshots or recordings count when they show that behavior. Redact credentials, IP addresses, private endpoints, and personal details. Updating the PR body should trigger review; otherwise ask a maintainer to comment @clawsweeper re-review. No stored-data contract changes require migration. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment `@clawsweeper re-review`.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the updated D1 regression exercises drainWebhooks with simulated fetch clients, while preserved earlier Node cancellation evidence does not connect real transport to the Worker drain and remainder recovery. Provide redacted local Worker logs or terminal output showing timeout, persisted cursor, healthy-subscriber progress, and later untouched-entry delivery; production deployment is unnecessary. Screenshots or recordings count when they show that behavior. Redact credentials, IP addresses, private endpoints, and personal details. Updating the PR body should trigger review; otherwise ask a maintainer to comment @clawsweeper re-review. No stored-data contract changes require migration. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment `@clawsweeper re-review`.
Evidence reviewed	9 items	Verified scope and parentage: The pinned merge-base-to-head delta contains 25 files. Raw head parents are current main followed by the earlier contributor head. Comparing the exact head with its current-main parent leaves only webhook source, its regression test, operator documentation, and one changelog entry; inherited CLI, dependency, and workflow changes are already on main. The stale test merge was not used. Still needed on current main: Current main awaits each subscriber POST without an abort signal. A subscriber that never returns can prevent progress to subsequent subscribers and cursor persistence. Application release and canonical check: The v0.5.0 application source also contains the unbounded POST. The bounded GitHub pull-list search found this PR and the distinct, open destination-security PR #75, with no merged webhook timeout replacement.
Findings	None	None.
Security	None	None.

How this fits together

Successful admin mutations append changes to D1 and schedule background webhook notifications. The drain sends changes to enabled subscribers and saves delivery cursors and status for the dashboard.

flowchart TD
  A[Admin mutation] --> B[D1 change log]
  A --> C[Background webhook drain]
  B --> C
  D[Subscriber URLs and cursors] --> C
  C --> E[POST with deadline]
  E --> F[Save attempted cursor and status]
  F --> G[Dashboard delivery result]
  F --> H[Untouched entries await next drain]

Before merge

Add real behavior proof - Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the updated D1 regression exercises drainWebhooks with simulated fetch clients, while preserved earlier Node cancellation evidence does not connect real transport to the Worker drain and remainder recovery. Provide redacted local Worker logs or terminal output showing timeout, persisted cursor, healthy-subscriber progress, and later untouched-entry delivery; production deployment is unnecessary. Screenshots or recordings count when they show that behavior. Redact credentials, IP addresses, private endpoints, and personal details. Updating the PR body should trigger review; otherwise ask a maintainer to comment @clawsweeper re-review. No stored-data contract changes require migration. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Complete next step (P2) - Add redacted real-transport evidence through the local production Worker drain demonstrating timeout persistence, healthy-subscriber progress, and later delivery of untouched entries.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	webhook production +29/-7; tests +90/-0	The production growth implements the stated deadline, while regression coverage verifies healthy subscribers and recovery.
Change relative to current main	4 files differ	The remaining 21 files in the pinned introduction delta already match current main.

Technical review

Best possible solution:

Keep the accepted notify-only deadline and preservation of untouched entries, supported by a real Worker transport trace demonstrating timeout recovery.

Do we have a high-confidence way to reproduce the issue?

Yes, source establishes the hang path: current main awaits an unbounded subscriber POST before advancing to other hooks. No failing runtime reproduction was executed during this read-only review.

Is this the best way to solve the issue?

Yes, the focused deadline and attempted-entry cursor tracking match the now-recorded delivery decision; the isolated regression addresses the previous race and remainder concerns.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 7be7204292c9.

Labels

Label changes:

No label changes.

Label justifications:

P2: This repairs bounded background notification reliability without demonstrated disruption to public flag evaluation.
merge-risk: 🚨 compatibility: Existing subscribers taking longer than two seconds lose their former unlimited wait; the repository administrator explicitly adopted and documented this tradeoff.
merge-risk: 🚨 message-delivery: A timed-out attempted notification is permanently skipped under the accepted notify-only policy, while untouched entries remain queued.
rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the updated D1 regression exercises drainWebhooks with simulated fetch clients, while preserved earlier Node cancellation evidence does not connect real transport to the Worker drain and remainder recovery. Provide redacted local Worker logs or terminal output showing timeout, persisted cursor, healthy-subscriber progress, and later untouched-entry delivery; production deployment is unnecessary. Screenshots or recordings count when they show that behavior. Redact credentials, IP addresses, private endpoints, and personal details. Updating the PR body should trigger review; otherwise ask a maintainer to comment @clawsweeper re-review. No stored-data contract changes require migration. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

Verified scope and parentage: The pinned merge-base-to-head delta contains 25 files. Raw head parents are current main followed by the earlier contributor head. Comparing the exact head with its current-main parent leaves only webhook source, its regression test, operator documentation, and one changelog entry; inherited CLI, dependency, and workflow changes are already on main. The stale test merge was not used. (apps/api/src/admin/webhooks.ts:181, 192ef4db835b)
Still needed on current main: Current main awaits each subscriber POST without an abort signal. A subscriber that never returns can prevent progress to subsequent subscribers and cursor persistence. (apps/api/src/admin/webhooks.ts:171, 7be7204292c9)
Application release and canonical check: The v0.5.0 application source also contains the unbounded POST. The bounded GitHub pull-list search found this PR and the distinct, open destination-security PR fix(admin): webhook create accepts private and metadata URLs #75, with no merged webhook timeout replacement. (apps/api/src/admin/webhooks.ts:171, 7be7204292c9)
Recorded deadline decision: The exact head authored by Peter Steinberger explicitly chooses two-second delivery, advancement only through attempted entries, and preservation of the remainder. GitHub independently verifies steipete has repository admin permission. This addresses the earlier request for owner acceptance: slow attempted notifications may be skipped under the documented notify-only contract, and that accepted tradeoff is retained rather than requested again. (apps/api/src/admin/webhooks.ts:181, 192ef4db835b)
Decision authority: GitHub returned permission admin for steipete, corroborating authority for the explicit delivery decision in the updated head.
Prior findings addressed: The new regression seeds D1 directly, attempts one hanging entry, delivers both entries to a healthy hook, then delivers only the untouched remainder after recovery and verifies no redelivery. CHANGELOG.md and docs/admin-console.md now document the timeout and cursor outcome. The hanging and recovered fetch clients remain simulated. (apps/api/test/webhooks.test.ts:131, 192ef4db835b)

Likely related people:

vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

Add a redacted real-transport trace through the local Worker drain showing timeout persistence, healthy-subscriber delivery, and untouched remainder recovery.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (32 earlier review cycles; latest 8 shown)

reviewed 2026-09-01T20:40:24.929Z sha e12a356 :: found issues before merge. :: [P1] Obtain approval for the two-second delivery cutoff | [P3] Add the required Unreleased changelog entry
reviewed 2026-09-03T07:57:00.477Z sha e12a356 :: blocked before merge. :: [P1] Preserve slow subscriber deliveries | [P3] Add the required Unreleased changelog entry
reviewed 2026-09-03T13:39:26.940Z sha e12a356 :: blocked before merge. :: [P1] Obtain approval for the two-second delivery cutoff | [P3] Document the new timeout behavior in Unreleased
reviewed 2026-09-09T19:21:19.348Z sha e12a356 :: needs real behavior proof before merge. :: [P1] Make the new delivery cutoff an explicit upgrade contract | [P2] Assert remaining entries instead of an empty second drain | [P3] Document the timeout and cursor outcome in Unreleased
reviewed 2026-09-11T16:37:17.984Z sha 0ad54b8 :: needs real behavior proof before merge. :: [P1] Make the new delivery cutoff an explicit upgrade contract | [P2] Assert remaining entries with an isolated drain fixture | [P3] Document the timeout and cursor outcome in Unreleased
reviewed 2026-09-18T09:53:29.144Z sha 0ad54b8 :: needs real behavior proof before merge. :: [P1] Resolve the delivery cutoff contract for existing subscribers | [P2] Assert remaining entries with an isolated drain fixture | [P3] Document the timeout and cursor outcome in Unreleased
reviewed 2026-10-02T10:42:48.014Z sha 0ad54b8 :: needs real behavior proof before merge. :: [P1] [P1] Make the delivery cutoff an explicit upgrade contract | [P2] [P2] Isolate pending entries and assert remainder delivery | [P3] [P3] Document the timeout behavior in Unreleased
reviewed 2026-10-04T14:26:49.351Z sha 0ad54b8 :: needs real behavior proof before merge. :: [P1] [P1] Resolve the delivery cutoff contract for existing subscribers | [P2] [P2] Isolate pending rows and assert remainder delivery | [P3] [P3] Document the timeout behavior in Unreleased

SebTardif · 2026-08-16T00:16:13Z

@clawsweeper re-review

clawsweeper · 2026-08-16T00:16:17Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

SebTardif · 2026-08-16T00:22:20Z

@clawsweeper re-review

clawsweeper · 2026-08-16T00:22:24Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

SebTardif · 2026-08-16T01:24:46Z

@clawsweeper

[P1] Preserve slow webhook deliveries across the new cutoff
A subscriber that responds just after two seconds now reaches the existing cursor-advance path and permanently misses the batch.

Pushed 63f6c5e. Timeout still aborts the hung POST. The cursor stays put so the next drain retries that batch. HTTP and unreachable outcomes still advance (notify-only).

@clawsweeper re-review

SebTardif · 2026-08-16T02:52:14Z

@clawsweeper

[P1] Get approval for retry-on-timeout delivery semantics
The new timeout path leaves the cursor unchanged, but the released webhook contract says failed posts are notify-only and never retried. A timeout can follow a completed remote side effect, so the next drain may duplicate entries; either preserve cursor advancement or obtain explicit approval.

Pushed e12a356. Contract chosen: notify-only, matching the released drain docs. Timeout now advances the cursor like HTTP and unreachable, then stops the batch.

@clawsweeper re-review

Claw P2 on c2f74de: each drain made two unbounded Cloudflare DoH requests before delivery. A stalled resolver could pin waitUntil. Each A and AAAA lookup now uses AbortSignal.timeout (2s), matching the openclaw#73 subscriber POST bound. Hung preflight is lastStatus timeout and leaves the cursor put. Tests cover signal presence and a 20ms hang-until-abort drain. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>

Each subscriber POST now carries AbortSignal.timeout so a stalled admin-supplied URL cannot pin waitUntil after POST /admin/*. Timeouts use the existing unreachable last_status path. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> (cherry picked from commit 2daa84b)

A fetch timeout now records last_status=timeout and leaves the cursor in place so the next drain retries. HTTP and unreachable failures still advance, matching the existing notify-only contract. Drop the release-owned changelog edit. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> (cherry picked from commit 28d1693)

Abort hung subscriber POSTs, record last_status=timeout, and advance the cursor like every other failed notification. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> (cherry picked from commit 0af4c1f)

A 2s cutoff still aborts a hung POST so drain can finish. Timeout no longer advances the cursor, so a slow subscriber is retried instead of permanently skipping the batch. HTTP and unreachable outcomes stay notify-only. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> (cherry picked from commit 63f6c5e)

The released drain contract advances the cursor on failed posts so subscribers are not retried. Timeout now matches HTTP and unreachable: advance, record timeout, stop the batch. That avoids duplicating a post that completed after the cutoff. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> (cherry picked from commit e12a356)

Claw P2 on c2f74de: each drain made two unbounded Cloudflare DoH requests before delivery. A stalled resolver could pin waitUntil. Each A and AAAA lookup now uses AbortSignal.timeout (2s), matching the openclaw#73 subscriber POST bound. Hung preflight is lastStatus timeout and leaves the cursor put. Tests cover signal presence and a 20ms hang-until-abort drain. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> (cherry picked from commit 9339996)

Co-authored-by: hannesrudolph <49103247+hannesrudolph@users.noreply.github.com>

…emplate docs: make PR descriptions plain-language first

Remove legacy Claude instruction filenames and aliases. Preserve retained AGENTS.md instructions and retire obsolete alias guidance where needed.

…aw#87) Refresh compatible dependencies and repair the Undici resolution Dependabot could not apply because Miniflare pinned vulnerable Undici 7.29.0. Development and deployment tooling now use patched Undici 7.29.1/8.10.2 and Sharp 0.35.4. The Node >=22.22.0 floor, React 18/19 SDK peer contract, and application/SDK versions are unchanged.

Bound CLI API requests and response-body reads to a new 30-second default deadline, with clear timeout errors and validated KRILLSWITCH_TIMEOUT_MS overrides from 0 through 2147483647 (0 restores unlimited waits). Clear timers on every exit path and cancel unread authentication-error bodies. Preserve existing HTTP errors, document upgrade and mutation-retry behavior in both CLI guides, and add regression coverage for timers, parsing bounds, and real HTTP body stalls. Validation: all documented checks and seven built-CLI HTTP scenarios passed on one AWS Crabbox lease; Codex autoreview clean through P2; exact-head CI green before merge. Co-authored-by: Sebastien Tardif <sebtardif@ncf.ca>

Bound subscriber delivery to two seconds, advance only attempted entries, and preserve the queued remainder. Verify timeout recovery and healthy subscribers with isolated D1 fixtures; update operator docs and the changelog. Co-authored-by: Sebastien Tardif <sebtardif@ncf.ca>

steipete

Approved: exact-head CI green and reviewed.

The merge-base changed after approval.

SebTardif requested a review from a team as a code owner August 16, 2026 00:08

clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Aug 16, 2026

SebTardif mentioned this pull request Aug 16, 2026

fix(admin): webhook create accepts private and metadata URLs #75

Open

SebTardif mentioned this pull request Sep 2, 2026

fix(auth): finish sign-in when GitHub membership stalls #91

Merged

SebTardif added 5 commits September 11, 2026 09:28

fix(admin): keep notify-only cursor after webhook timeout

1cf5de7

Abort hung subscriber POSTs, record last_status=timeout, and advance the cursor like every other failed notification. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> (cherry picked from commit 0af4c1f)

SebTardif force-pushed the fix/webhook-drain-timeout branch from e12a356 to 0ad54b8 Compare September 11, 2026 16:29

roboclaw-bot and others added 6 commits September 12, 2026 19:40

docs: make PR descriptions plain-language first

bcb5988

Co-authored-by: hannesrudolph <49103247+hannesrudolph@users.noreply.github.com>

Merge pull request openclaw#100 from openclaw/docs/plain-english-pr-t…

2a70333

…emplate docs: make PR descriptions plain-language first

chore: retire CLAUDE.md files

4e172a0

Remove legacy Claude instruction filenames and aliases. Preserve retained AGENTS.md instructions and retire obsolete alias guidance where needed.

steipete requested a review from a team as a code owner October 7, 2026 03:52

steipete requested a review from vincentkoc October 7, 2026 03:52

steipete changed the title ~~fix(admin): abort hung webhook drain fetches~~ fix(admin): unblock webhook drains when subscribers stall Oct 7, 2026

steipete previously approved these changes Oct 7, 2026

View reviewed changes

steipete merged commit 2858001 into openclaw:main Oct 7, 2026
10 checks passed

Conversation

SebTardif commented Aug 16, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

User Impact

Why This Change Was Made

Evidence

Uh oh!

clawsweeper Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ClawSweeper review complete

Uh oh!

clawsweeper Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ClawSweeper review

What this changes

Merge readiness

Review scores

Verification

How this fits together

Before merge

Security

Review metrics

Technical review

Labels

Evidence

Rank-up moves

Rating scale

Workflow

History

Uh oh!

SebTardif commented Aug 16, 2026

Uh oh!

clawsweeper Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SebTardif commented Aug 16, 2026

Uh oh!

clawsweeper Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SebTardif commented Aug 16, 2026

Uh oh!

SebTardif commented Aug 16, 2026

Uh oh!

steipete left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SebTardif commented Aug 16, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Aug 16, 2026 •

edited

Loading

clawsweeper Bot commented Aug 16, 2026 •

edited

Loading

clawsweeper Bot commented Aug 16, 2026 •

edited

Loading

clawsweeper Bot commented Aug 16, 2026 •

edited

Loading